feat(avm): remove rethrowable reverts hack #9752
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @fcarreiro and the rest of your teammates on |
fcarreiro
force-pushed
the
fc/avm-remove-reverts-hack
branch
from
da2f733
to
2bf5a72
Compare
fcarreiro
commented
Nov 5, 2024
| @@ -37,112 +34,6 @@ export interface ACIRExecutionResult { | |||
| returnWitness: ACVMWitness; | |||
| } | |||
|
|
|||
| /** | |||
There was a problem hiding this comment.
Moving to "simulator/common"
Changes to public function bytecode sizes
🧾 Summary (100% most significant diffs)
Full diff report 👇
|
fcarreiro
force-pushed
the
fc/avm-remove-reverts-hack
branch
from
7997888
to
6743e0e
Compare
Maddiaa0
pushed a commit
that referenced
this pull request
Nov 6, 2024
This PR removes the RethrowableError hack in the AVM simulator, and relies on the PublicContext's [rethrow](https://github.com/AztecProtocol/aztec-packages/blob/master/noir-projects/aztec-nr/aztec/src/context/public_context.nr#L88) to propagate the errors. There are two caveats. First, because currently Aztec-nr does not keep track of the cause chain, it would be impossible to have the call stack and original contract address available, so that the PXE can interpret the error and show debug information. Solidity has the same problem. I'm introducing a heuristic to keep track of the call stack for the simple case where the caller always rethrows: the simulator keeps a running trace in the machineState, and the caller uses this trace IF the revertData coincides. That is, if you are (re)throwing the same as what we were tracking. Second, while this all works well for errors in user code (e.g., `assert` in Noir), because they generate a revertData with an error selector and data, the "intrinsic" errors from the simulator (aka exceptional halts) do not work as well. E.g., "division by zero", "duplicated nullifier", "l1 to l2 blah blah". These errors are exceptions in typescript and do not have an associated error selector, and do not add to the revertdata. This _could_ be done with enshrined error selectors. That's easy in the simulator, but it's not easy in the circuit for several reasons that are beyond the scope of this description. The current solution is to propagate the error message (the user will see the right error) but you cannot "catch and process" the error in aztec.nr because there is no selector. This is not a limitation right now because there's no interface in the PublicContext that would let you catch errors. To be continued. Part of #9061.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR removes the RethrowableError hack in the AVM simulator, and relies on the PublicContext's rethrow to propagate the errors. There are two caveats.
First, because currently Aztec-nr does not keep track of the cause chain, it would be impossible to have the call stack and original contract address available, so that the PXE can interpret the error and show debug information. Solidity has the same problem. I'm introducing a heuristic to keep track of the call stack for the simple case where the caller always rethrows: the simulator keeps a running trace in the machineState, and the caller uses this trace IF the revertData coincides. That is, if you are (re)throwing the same as what we were tracking.
Second, while this all works well for errors in user code (e.g.,
assertin Noir), because they generate a revertData with an error selector and data, the "intrinsic" errors from the simulator (aka exceptional halts) do not work as well. E.g., "division by zero", "duplicated nullifier", "l1 to l2 blah blah". These errors are exceptions in typescript and do not have an associated error selector, and do not add to the revertdata. This could be done with enshrined error selectors. That's easy in the simulator, but it's not easy in the circuit for several reasons that are beyond the scope of this description. The current solution is to propagate the error message (the user will see the right error) but you cannot "catch and process" the error in aztec.nr because there is no selector. This is not a limitation right now because there's no interface in the PublicContext that would let you catch errors. To be continued.Part of #9061.